feat(tooling): migrate docs-builder CLI from ConsoleAppFramework to Nullean.Argh#3202
Open
feat(tooling): migrate docs-builder CLI from ConsoleAppFramework to Nullean.Argh#3202
Conversation
…rk to Nullean.Argh 0.7.0 Replaces ConsoleAppFramework with Nullean.Argh.Hosting in docs-builder and Nullean.Argh (standalone) in the aspire AppHost. Key improvements: proper namespace hierarchy (assembler, codex, etc.), record binding via [AsParameters] for the shared ElasticsearchIndexOptions, ICommandMiddleware replacing filter chain, GlobalCliOptions with --log-level/--config-source/--skip-private-repositories as typed global flags, and docs-builder assemble as a hoisted root command. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Create IsolatedBuildOptions, AssemblerBuildOptions, and AssemblerCloneOptions records in the service layer. Update all six affected service methods to accept these DTOs and the existing ElasticsearchIndexOptions directly, eliminating the tuple-based state unpacking in ServiceInvoker lambdas. Commands are now thin wrappers that construct a DTO and pass it straight through. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…m> in DTOs
- Bump Nullean.Argh + Nullean.Argh.Hosting to 0.8.0
- Delete ExporterParser: IReadOnlySet<Exporter>? now bound natively by argh
- IsolatedBuildOptions and AssemblerBuildOptions include Exporters via repeated
flags (--exporters Html --exporters Elasticsearch); [CollectionSyntax(Separator=",")]
deferred until argh fixes [AsParameters] + IReadOnlySet<Enum> interaction
- IsolatedBuildCommand.Build and AssemblerCommands.Build use [AsParameters] DTO
- [CommandName("build")] + [DefaultCommand] ready for MapAndRootAlias once the
generator bug (missing { on subsequent methods) is fixed in argh
- Map<IsolatedBuildCommand>() used in place of MapAndRootAlias<> for now
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…s-builder build
- Bump to 0.8.1 (fixes MapAndRootAlias generator bug — missing { on subsequent methods)
- Switch Map<IsolatedBuildCommand>() → MapAndRootAlias<IsolatedBuildCommand>():
docs-builder build is now both a named command and the root default alias
- IsolatedBuildOptions and AssemblerBuildOptions include IReadOnlySet<Exporter>?
(repeated flags: --exporters Html --exporters Elasticsearch)
- [CollectionSyntax(Separator=",")] deferred: [AsParameters] + [CollectionSyntax]
still emits a stray closing brace in 0.8.1 generated code
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Bump to 0.9.0 (fixes [AsParameters] + [CollectionSyntax] stray-brace generator bug) - Re-enable [CollectionSyntax(Separator=",")] on IsolatedBuildOptions.Exporters and AssemblerBuildOptions.Exporters: --exporters Html,Elasticsearch now works - Add ArghApp.TryArghIntrinsicCommand(args) pre-host fast path: --help, --version, __schema and __completion no longer trigger host construction or startup logs - Nullean.Argh.Interfaces added to Isolated and Assembler service projects Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
ElasticsearchIndexOptions:
- Endpoint/ProxyAddress: string? -> Uri? with [UriScheme("http","https")]
- BootstrapTimeout: int? (minutes) -> TimeSpan? with [TimeSpanRange("1s","60m")]
- [Range] on SearchNumThreads, IndexNumThreads, BufferSize, MaxRetries
- Rename NoAiEnrichment -> AiEnrichment, NoEis -> Eis (positive flags,
clean --no-ai-enrichment / --no-eis negation; removes double-negative)
- Full XML docs on all properties
XML documentation:
- IsolatedBuildOptions, AssemblerBuildOptions: XML docs on all properties
- Namespace class summaries and remarks: assembler (navigation.yml unified site),
codex (independent per-set navigation), inbound-links (link registry concept)
- All assembler, codex, and inbound-links commands: complete <summary> and
<remarks> explaining concepts (assembler, codex, link registry, bloom filter),
ordering requirements, and usage examples
- Root commands (build, index, diff): <summary> tightened, <remarks> added
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…pace summaries - ChangelogCommands: add class-level summary; rewrite all method summaries to one terse sentence; move detail to <remarks>; remove <code> invocation blocks - All commands: remove <code> CLI invocation examples from <remarks> throughout - GlobalCliOptions: remove manual enum listing from --log-level summary (argh renders allowed values automatically) - AssemblerBuildOptions, IsolatedBuildOptions: add XML docs to properties (visible in IDE; argh cross-assembly doc limitation tracked separately) - Nested assembler namespaces: add class-level <summary> to BloomFilterCommands, ConfigurationCommand, ContentSourceCommands, DeployCommands, NavigationCommands so they appear in assembler --help listings - ServeCommand, MoveCommand, FormatCommand, IndexCommand: tighten summaries and move detail to <remarks> without code blocks Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
… docs from referenced projects PR #25 in 0.9.1 adds XML doc fallback for [AsParameters] DTO members in referenced assemblies. Enable GenerateDocumentationFile on Isolated, Assembler, and Configuration service projects so argh's generator can read property summaries from the produced .xml files. Result: docs-builder build --help, assembler build --help, assembler index --help etc. now show full descriptions for every [AsParameters] DTO property (IsolatedBuildOptions, AssemblerBuildOptions, ElasticsearchIndexOptions). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…alBaseUrl as Uri?
- IsolatedBuildOptions.CanonicalBaseUrl: string? -> Uri? with [Url]
(argh maps [Url] on Uri? to [UriScheme("http","https")] constraint)
- IsolatedBuildService.Build: remove manual Uri.TryCreate; use the
already-validated Uri? from argh binding with ?? default fallback
- ElasticsearchIndexOptions.Endpoint/ProxyAddress: [UriScheme("http","https")]
-> [Url] for brevity (equivalent per argh generator)
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…ectoryInfo
- ElasticsearchIndexOptions.CertificatePath: string? -> FileInfo? [FileExtensions("pem,der,crt,cer")]
- IsolatedBuildOptions.Path, Output: string? -> DirectoryInfo?
- Codex config params (all four commands): string -> FileInfo [FileExtensions("yml,yaml")]
- Codex output params: string? -> DirectoryInfo?
- DeployCommands: planFile -> FileInfo [json], @out -> FileInfo?, redirectsFile -> FileInfo? [json]
- CodexUpdateRedirectsCommand.redirectsFile: string? -> FileInfo? [json]
- NavigationCommands.ValidateLinkReference file: string? -> FileInfo? [json]
- InboundLinkCommands.ValidateLinkReference file: string? -> FileInfo? [json]
- BloomFilterCommands.Create builtDocsDir: string -> DirectoryInfo
- AssemblerCommands.Serve path: string? -> DirectoryInfo?
- ServeCommand path: string? -> DirectoryInfo?
- ChangelogCommand.Init path/changelogDir/bundlesDir: string? -> DirectoryInfo?
- ChangelogCommand config params (Add, Bundle, Remove, Render, GhRelease, Upload, EvaluatePr): string? -> FileInfo? [yml,yaml]
- ChangelogCommand directory params: string? -> DirectoryInfo?
- ChangelogCommand.BundleAmend bundlePath: string -> FileInfo [yml,yaml]
- Help text: path/file/dir params now show as <dir>, <file>, <path> with extension hints
Tilde expansion report (params needing future argh [ExpandTilde] attribute):
changelog init path/changelogDir/bundlesDir, all config params, all directory
params, assembler deploy planFile/@out, CertificatePath, build Path/Output.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…tSymbolicLinks], [ExpandUserProfile] - Bump Nullean.Argh, Nullean.Argh.Hosting, Nullean.Argh.Interfaces to 0.11.0 - [Existing] on all input FileInfo/DirectoryInfo params that must be present: CertificatePath, all codex config files, changelog config/bundlePath, deploy planFile/redirectsFile, navigation/inbound-links file, bloom-filter builtDocsDir, serve path params, IsolatedBuildOptions.Path (source dir must exist) - [RejectSymbolicLinks] on every FileInfo/DirectoryInfo param and property - [ExpandUserProfile] on every FileInfo/DirectoryInfo param and property (fixes ~/path expansion for all path arguments) - IsolatedBuildOptions.Output intentionally omitted from [Existing] (created by build) - Codex and changelog output directories omitted from [Existing] (created by commands) Help output now shows: [existing] [no symlinks] [expand ~ profile] on appropriate args. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Fixes: - PR #28: DTO XML summaries now render in --help; global options show short flag first (e.g. -l, --log-level) - PR #29: enum choices listed lowercase in help (html, elasticsearch, ...) - PR #30: [Existing] on optional nullable FileInfo?/DirectoryInfo? no longer throws ArgumentNullException when the flag is omitted - PR #31: MapAndRootAlias root help and leading flag handling fixed Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…Parameters] Argh 0.12.0 correctly handles IReadOnlySet<T> in [AsParameters] DTOs but initialises the collection to an empty HashSet (not null) when no --exporters flags are supplied. The previous `??=` null-coalesce never fired on an empty set, so IsolatedBuildService and AssemblerBuildService ran with zero exporters (producing no output, no links.json). Replace `??=` with an explicit count-based guard in both services. Also enable [CollectionSyntax(Separator=",")] on IsolatedBuildOptions.Exporters now that the [AsParameters] + [CollectionSyntax] generator bug is fixed in 0.12.0. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…ed code 0.12.1 (PR #33) fixed IReadOnlySet<T>? defaulting to null when omitted in [AsParameters] DTOs, but the generator emits the nullable null-return into a non-nullable typed local variable (CS8600 / missing ? on the declaration), making the project unbuildable. 0.12.2 does not fix this. Pin to 0.12.0 and keep the is not { Count: > 0 } guard as a workaround until the generator bug is resolved upstream. Remove [CollectionSyntax] comments from DTOs (they were only relevant to the broken [AsParameters]+[CollectionSyntax] combination which is now fixed in 0.12.0 anyway). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Fixes the CS8600 generator bug introduced in 0.12.1 where IReadOnlySet<T>? in
[AsParameters] DTOs was emitted with a non-nullable local type. Remove the
is not { Count: > 0 } workaround — ??= is correct again. Re-enable
[CollectionSyntax(Separator=",")] on IsolatedBuildOptions and AssemblerBuildOptions.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…s in help, AGH0033)
Contributor
📝 WalkthroughWalkthroughThis pull request migrates the documentation tooling CLI from ConsoleAppFramework to Nullean.Argh, refactoring command handling, parameter processing, and middleware patterns. The changes introduce strongly-typed option records (AssemblerBuildOptions, IsolatedBuildOptions, ElasticsearchIndexOptions, etc.) to replace scattered nullable parameters. Command definitions are updated to use Argh attributes, commands are reorganized with middleware instead of filters, and cancellation is standardized from 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
This was referenced Apr 29, 2026
Member
Author
|
@copilot resolve the merge conflicts in this pull request |
# Conflicts: # src/tooling/docs-builder/Commands/ChangelogCommand.cs Co-authored-by: Mpdreamz <245275+Mpdreamz@users.noreply.github.com>
Contributor
Resolved in 9706e56. The only conflict was in |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/tooling/docs-builder/Commands/Assembler/ContentSourceCommands.cs (1)
51-56:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn the
ShouldBuildresult instead of hard-coding success.
ShouldBuild(...)is awaited and discarded here, somatchwill always exit 0 even when the repository/tag should be rejected.♻️ Suggested fix
serviceInvoker.AddCommand(service, (repository, branchOrTag), static async (s, collector, state, ctx) => { - _ = await s.ShouldBuild(collector, state.repository, state.branchOrTag, ctx); - return true; + return await s.ShouldBuild(collector, state.repository, state.branchOrTag, ctx); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tooling/docs-builder/Commands/Assembler/ContentSourceCommands.cs` around lines 51 - 56, The lambda passed to serviceInvoker.AddCommand currently awaits and discards the result of s.ShouldBuild, causing the command to always return true; change the lambda in the AddCommand call so it returns the awaited boolean result from s.ShouldBuild(...) instead of returning true — i.e., await and return the value from ShouldBuild inside the static async (s, collector, state, ctx) => { ... } block so the command's exit status reflects the ShouldBuild decision.src/services/Elastic.Documentation.Assembler/Building/AssemblerBuildService.cs (1)
55-55:⚠️ Potential issue | 🔴 Critical
SetEqualswill fail at runtime ifexporterscomes fromoptions.ExporterswithoutDocumentationState.
AssemblerBuildOptions.ExportersisIReadOnlySet<Exporter>?. At line 48, the??=only assigns if null; otherwiseexportersretains theIReadOnlySettype. If the condition at line 50 is false (noDocumentationStateto strip), line 53 callsSetEqualson anIReadOnlySet, which lacks this method in the BCL.♻️ Suggested fix
- var elasticsearchExportOnly = exporters.SetEquals([Exporter.Elasticsearch]); + var elasticsearchExportOnly = + exporters.Count == 1 && exporters.Contains(Exporter.Elasticsearch);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/Elastic.Documentation.Assembler/Building/AssemblerBuildService.cs` at line 55, The code calls exporters.SetEquals(...) but exporters is an IReadOnlySet<Exporter>? (AssemblerBuildOptions.Exporters) so SetEquals will not exist at runtime; fix by ensuring exporters is an ISet<Exporter> before calling SetEquals—e.g., if you already create or mutate exporters when handling DocumentationState, construct a concrete HashSet<Exporter> from options.Exporters (or wrap null as empty) and then call thatHashSet.SetEquals(new[]{Exporter.Elasticsearch}); update references to the exporters variable used in the equality check (the exporters local and the Exporter.Elasticsearch check) so the concrete SetEquals method is available.src/tooling/docs-builder/Middleware/CatchExceptionMiddleware.cs (1)
18-22:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCancel the console event, not just the token.
This handler discards
ConsoleCancelEventArgs, so Ctrl+C can still take down the process before the cancellation path runs.🔧 Proposed fix
- Console.CancelKeyPress += (_, _) => + Console.CancelKeyPress += (_, e) => { logger.LogInformation("Received CTRL+C cancelling"); + e.Cancel = true; _cancelKeyPressed = true; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tooling/docs-builder/Middleware/CatchExceptionMiddleware.cs` around lines 18 - 22, The Console.CancelKeyPress handler currently ignores the ConsoleCancelEventArgs which allows the process to be terminated by Ctrl+C; update the handler attached to Console.CancelKeyPress so it accepts the ConsoleCancelEventArgs parameter (e.g., (sender, args) => { ... }), set args.Cancel = true to prevent the OS from terminating the process, and keep the existing behavior of logging via logger.LogInformation and setting _cancelKeyPressed = true so the graceful cancellation path runs (refer to the Console.CancelKeyPress subscription, the lambda handler, logger.LogInformation, and the _cancelKeyPressed flag).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/Elastic.Documentation.Configuration/ElasticsearchEndpointConfigurator.cs`:
- Around line 142-147: The code calls ReadAllBytesAsync and
X509CertificateLoader.LoadCertificate even when the certificate path is missing
because EmitGlobalError in the options.CertificatePath check doesn't stop
execution; update the block around options.CertificatePath to short-circuit
after collector.EmitGlobalError (e.g., return from the method or skip the
certificate loading path) so that fileSystem.File.ReadAllBytesAsync and
X509CertificateLoader.LoadCertificate are not invoked when
fileSystem.File.Exists(options.CertificatePath.FullName) is false; keep the same
error emission using collector.EmitGlobalError and only proceed to call
ReadAllBytesAsync and assign cfg.Certificate when the file exists.
- Around line 150-152: The code truncates sub-minute TimeSpan values when
assigning options.BootstrapTimeout to cfg.BootstrapTimeout via
(int)options.BootstrapTimeout.Value.TotalMinutes; preserve precision by storing
seconds instead: compute (int)options.BootstrapTimeout.Value.TotalSeconds (or
change cfg.BootstrapTimeout to a TimeSpan) so inputs like "30s" or "90s" are
represented accurately; update any downstream consumers or property
name/comments for cfg.BootstrapTimeout to reflect that the value is in seconds
if you choose the seconds approach.
In `@src/Elastic.Documentation/GlobalCommandLine.cs`:
- Around line 23-27: The parsing branch for the
--config-source/--configuration-source/-c flag currently ignores invalid values
because ConfigurationSourceExtensions.TryParse silently fails; update the branch
in GlobalCommandLine.cs so that if TryParse(args[i + 1], out var cs, true, true)
returns false you reject the input (e.g., log a clear error and exit or throw a
UsageException) instead of leaving the default, otherwise continue setting
options = options with { ConfigurationSource = cs } and incrementing i; this
change ensures invalid config-source values are surfaced immediately.
In `@src/services/Elastic.Documentation.Isolated/IsolatedBuildService.cs`:
- Around line 77-78: Restore the absolute-URI validation when assigning
canonicalBaseUri in IsolatedBuildService (so BuildContext never gets relative
Uris): perform a Uri.TryCreate(inputValue, UriKind.Absolute, out var
canonicalBaseUri) (or validate the incoming canonicalBaseUri parameter with
UriKind.Absolute) and only set canonicalBaseUri to the default new
Uri("https://docs-v3-preview.elastic.dev") or the parsed Uri when the TryCreate
succeeds; ensure callers that pass invalid/relative values are rejected or fall
back to the absolute default so BuildContext only ever receives absolute Uris.
In `@src/tooling/docs-builder/Commands/ChangelogCommand.cs`:
- Around line 87-89: The default changelog and bundles directories are being
resolved from the current working directory instead of the repository docs
folder; update the logic in ChangelogCommand where configPath, changelogPath and
bundlesPath are computed so that when changelogDir or bundlesDir are null you
join them to docsFolder.FullName (e.g. use
_fileSystem.Path.Join(docsFolder.FullName, "changelog") and
_fileSystem.Path.Join(docsFolder.FullName, "releases")) so the default paths
match where changelog.yml is written; apply the same fix to the other occurrence
mentioned (the block around lines 185-205) so all defaults consistently resolve
relative to docsFolder rather than cwd.
In `@src/tooling/docs-builder/Commands/InboundLinkCommands.cs`:
- Around line 59-65: The [Existing] validation on the ValidateLinkReference
method's file parameter blocks null so the fallback in CheckWithLocalLinksJson
never runs; edit ValidateLinkReference to allow null by removing the [Existing]
attribute (or replace it with an attribute variant that permits null if
available) so that ValidateLinkReference(FileInfo? file = null, ...) can pass a
null through to _linkIndexService.CheckWithLocalLinksJson which will apply file
??= ".artifacts/docs/html/links.json"; ensure the parameter signature and any
related attribute usages on ValidateLinkReference are updated accordingly.
In `@src/tooling/docs-builder/Middleware/CatchExceptionMiddleware.cs`:
- Around line 34-37: Await the collector startup before emitting or stopping it:
change the fire-and-forget call to await
collector.StartAsync(context.CancellationToken) so that EmitGlobalError and
StopAsync run only after StartAsync completes; ensure you still await StopAsync
afterwards and preserve setting context.ExitCode = 1, referencing
collector.StartAsync, collector.EmitGlobalError, collector.StopAsync and
context.ExitCode.
In `@src/tooling/docs-builder/Middleware/CheckForUpdatesMiddleware.cs`:
- Around line 27-31: GetLatestVersion can throw and HttpClient/response are not
disposed, so make the update check best-effort: wrap the call to
GetLatestVersion (and the subsequent CompareWithAssemblyVersion) in a try/catch
that logs the exception (use _logger.LogDebug or LogWarning) but does not
rethrow, ensuring the middleware does not fail the command; inside
GetLatestVersion ensure HTTP resources are disposed (use using/using var for
HttpClient and HttpResponseMessage and dispose any streams/readers used) and
catch file/IO/HTTP exceptions there as well or let them bubble to the outer try
so they are logged and ignored rather than breaking execution.
---
Outside diff comments:
In
`@src/services/Elastic.Documentation.Assembler/Building/AssemblerBuildService.cs`:
- Line 55: The code calls exporters.SetEquals(...) but exporters is an
IReadOnlySet<Exporter>? (AssemblerBuildOptions.Exporters) so SetEquals will not
exist at runtime; fix by ensuring exporters is an ISet<Exporter> before calling
SetEquals—e.g., if you already create or mutate exporters when handling
DocumentationState, construct a concrete HashSet<Exporter> from
options.Exporters (or wrap null as empty) and then call
thatHashSet.SetEquals(new[]{Exporter.Elasticsearch}); update references to the
exporters variable used in the equality check (the exporters local and the
Exporter.Elasticsearch check) so the concrete SetEquals method is available.
In `@src/tooling/docs-builder/Commands/Assembler/ContentSourceCommands.cs`:
- Around line 51-56: The lambda passed to serviceInvoker.AddCommand currently
awaits and discards the result of s.ShouldBuild, causing the command to always
return true; change the lambda in the AddCommand call so it returns the awaited
boolean result from s.ShouldBuild(...) instead of returning true — i.e., await
and return the value from ShouldBuild inside the static async (s, collector,
state, ctx) => { ... } block so the command's exit status reflects the
ShouldBuild decision.
In `@src/tooling/docs-builder/Middleware/CatchExceptionMiddleware.cs`:
- Around line 18-22: The Console.CancelKeyPress handler currently ignores the
ConsoleCancelEventArgs which allows the process to be terminated by Ctrl+C;
update the handler attached to Console.CancelKeyPress so it accepts the
ConsoleCancelEventArgs parameter (e.g., (sender, args) => { ... }), set
args.Cancel = true to prevent the OS from terminating the process, and keep the
existing behavior of logging via logger.LogInformation and setting
_cancelKeyPressed = true so the graceful cancellation path runs (refer to the
Console.CancelKeyPress subscription, the lambda handler, logger.LogInformation,
and the _cancelKeyPressed flag).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b408d686-9799-4f94-9392-3c097980ab2f
📒 Files selected for processing (51)
Directory.Packages.propsaspire/AppHost.csaspire/aspire.csprojsrc/Elastic.Documentation.Configuration/Elastic.Documentation.Configuration.csprojsrc/Elastic.Documentation.Configuration/ElasticsearchEndpointConfigurator.cssrc/Elastic.Documentation.ServiceDefaults/AppDefaultsExtensions.cssrc/Elastic.Documentation/GlobalCommandLine.cssrc/services/Elastic.Documentation.Assembler/Building/AssemblerBuildOptions.cssrc/services/Elastic.Documentation.Assembler/Building/AssemblerBuildService.cssrc/services/Elastic.Documentation.Assembler/Building/AssemblerSitemapService.cssrc/services/Elastic.Documentation.Assembler/Elastic.Documentation.Assembler.csprojsrc/services/Elastic.Documentation.Assembler/Indexing/AssemblerIndexService.cssrc/services/Elastic.Documentation.Assembler/Sourcing/AssemblerCloneOptions.cssrc/services/Elastic.Documentation.Assembler/Sourcing/AssemblerCloneService.cssrc/services/Elastic.Documentation.Isolated/Elastic.Documentation.Isolated.csprojsrc/services/Elastic.Documentation.Isolated/IsolatedBuildOptions.cssrc/services/Elastic.Documentation.Isolated/IsolatedBuildService.cssrc/services/Elastic.Documentation.Isolated/IsolatedIndexService.cssrc/tooling/docs-builder/Arguments/ExportOption.cssrc/tooling/docs-builder/Arguments/ProductInfoParser.cssrc/tooling/docs-builder/Commands/Assembler/AssemblerCommands.cssrc/tooling/docs-builder/Commands/Assembler/AssemblerIndexCommand.cssrc/tooling/docs-builder/Commands/Assembler/AssemblerSitemapCommand.cssrc/tooling/docs-builder/Commands/Assembler/BloomFilterCommands.cssrc/tooling/docs-builder/Commands/Assembler/ConfigurationCommands.cssrc/tooling/docs-builder/Commands/Assembler/ContentSourceCommands.cssrc/tooling/docs-builder/Commands/Assembler/DeployCommands.cssrc/tooling/docs-builder/Commands/Assembler/NavigationCommands.cssrc/tooling/docs-builder/Commands/ChangelogCommand.cssrc/tooling/docs-builder/Commands/Codex/CodexCommands.cssrc/tooling/docs-builder/Commands/Codex/CodexIndexCommand.cssrc/tooling/docs-builder/Commands/Codex/CodexUpdateRedirectsCommand.cssrc/tooling/docs-builder/Commands/DiffCommands.cssrc/tooling/docs-builder/Commands/FormatCommand.cssrc/tooling/docs-builder/Commands/InboundLinkCommands.cssrc/tooling/docs-builder/Commands/IndexCommand.cssrc/tooling/docs-builder/Commands/IsolatedBuildCommand.cssrc/tooling/docs-builder/Commands/MoveCommand.cssrc/tooling/docs-builder/Commands/ServeCommand.cssrc/tooling/docs-builder/DocumentationTooling.cssrc/tooling/docs-builder/Filters/InfoLoggerFilter.cssrc/tooling/docs-builder/Filters/ReplaceLogFilter.cssrc/tooling/docs-builder/Filters/StopwatchFilter.cssrc/tooling/docs-builder/GlobalCliOptions.cssrc/tooling/docs-builder/Http/InMemoryBuildState.cssrc/tooling/docs-builder/Middleware/CatchExceptionMiddleware.cssrc/tooling/docs-builder/Middleware/CheckForUpdatesMiddleware.cssrc/tooling/docs-builder/Middleware/InfoLoggerMiddleware.cssrc/tooling/docs-builder/Middleware/StopwatchMiddleware.cssrc/tooling/docs-builder/Program.cssrc/tooling/docs-builder/docs-builder.csproj
💤 Files with no reviewable changes (6)
- src/tooling/docs-builder/DocumentationTooling.cs
- src/tooling/docs-builder/Filters/StopwatchFilter.cs
- src/tooling/docs-builder/Filters/InfoLoggerFilter.cs
- src/tooling/docs-builder/Filters/ReplaceLogFilter.cs
- src/tooling/docs-builder/Arguments/ExportOption.cs
- src/tooling/docs-builder/Commands/FormatCommand.cs
Comment on lines
+142
to
+147
| if (options.CertificatePath is not null) | ||
| { | ||
| if (!fileSystem.File.Exists(options.CertificatePath)) | ||
| collector.EmitGlobalError($"'{options.CertificatePath}' does not exist"); | ||
| var bytes = await fileSystem.File.ReadAllBytesAsync(options.CertificatePath, ctx); | ||
| var loader = X509CertificateLoader.LoadCertificate(bytes); | ||
| cfg.Certificate = loader; | ||
| if (!fileSystem.File.Exists(options.CertificatePath.FullName)) | ||
| collector.EmitGlobalError($"'{options.CertificatePath.FullName}' does not exist"); | ||
| var bytes = await fileSystem.File.ReadAllBytesAsync(options.CertificatePath.FullName, ctx); | ||
| cfg.Certificate = X509CertificateLoader.LoadCertificate(bytes); |
Contributor
There was a problem hiding this comment.
Stop after reporting a missing certificate file.
EmitGlobalError on Line 145 does not short-circuit anything here, so the code still calls ReadAllBytesAsync and LoadCertificate on the missing path. That turns a clean validation failure into an exception path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Elastic.Documentation.Configuration/ElasticsearchEndpointConfigurator.cs`
around lines 142 - 147, The code calls ReadAllBytesAsync and
X509CertificateLoader.LoadCertificate even when the certificate path is missing
because EmitGlobalError in the options.CertificatePath check doesn't stop
execution; update the block around options.CertificatePath to short-circuit
after collector.EmitGlobalError (e.g., return from the method or skip the
certificate loading path) so that fileSystem.File.ReadAllBytesAsync and
X509CertificateLoader.LoadCertificate are not invoked when
fileSystem.File.Exists(options.CertificatePath.FullName) is false; keep the same
error emission using collector.EmitGlobalError and only proceed to call
ReadAllBytesAsync and assign cfg.Certificate when the file exists.
Comment on lines
150
to
+152
| cfg.CertificateIsNotRoot = options.CertificateNotRoot.Value; | ||
| if (options.BootstrapTimeout.HasValue) | ||
| cfg.BootstrapTimeout = options.BootstrapTimeout.Value; | ||
|
|
||
| if (options.NoAiEnrichment == true) | ||
| cfg.BootstrapTimeout = (int)options.BootstrapTimeout.Value.TotalMinutes; |
Contributor
There was a problem hiding this comment.
BootstrapTimeout silently truncates second-based inputs.
The CLI now accepts values like 90s, but (int)TotalMinutes turns 30s into 0 and 90s into 1. Either preserve TimeSpan precision downstream or reject sub-minute inputs; the current conversion changes the user's value without warning.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Elastic.Documentation.Configuration/ElasticsearchEndpointConfigurator.cs`
around lines 150 - 152, The code truncates sub-minute TimeSpan values when
assigning options.BootstrapTimeout to cfg.BootstrapTimeout via
(int)options.BootstrapTimeout.Value.TotalMinutes; preserve precision by storing
seconds instead: compute (int)options.BootstrapTimeout.Value.TotalSeconds (or
change cfg.BootstrapTimeout to a TimeSpan) so inputs like "30s" or "90s" are
represented accurately; update any downstream consumers or property
name/comments for cfg.BootstrapTimeout to reflect that the value is in seconds
if you choose the seconds approach.
Comment on lines
+23
to
27
| else if (args[i] is "--config-source" or "--configuration-source" or "-c" && i + 1 < args.Length) | ||
| { | ||
| if (args.Length > i + 1) | ||
| { | ||
| cli = cli with { LogLevel = GetLogLevel(args[i + 1]) }; | ||
| globalArgs.Add("--log-level"); | ||
| globalArgs.Add(args[i + 1]); | ||
| } | ||
| i++; | ||
| } | ||
| else if (args[i] is "--config-source" or "--configuration-source" or "-c") | ||
| { | ||
| if (args.Length > i + 1 && ConfigurationSourceExtensions.TryParse(args[i + 1], out var cs, true, true)) | ||
| { | ||
| cli = cli with { ConfigurationSource = cs }; | ||
| globalArgs.Add("--config-source"); | ||
| globalArgs.Add(args[i + 1]); | ||
| } | ||
| if (ConfigurationSourceExtensions.TryParse(args[i + 1], out var cs, true, true)) | ||
| options = options with { ConfigurationSource = cs }; | ||
| i++; |
Contributor
There was a problem hiding this comment.
Reject invalid --config-source values.
An explicit bad source currently leaves the default in place, which can make the app read from the wrong configuration source without any signal.
🔧 Proposed fix
- else if (args[i] is "--config-source" or "--configuration-source" or "-c" && i + 1 < args.Length)
- {
- if (ConfigurationSourceExtensions.TryParse(args[i + 1], out var cs, true, true))
- options = options with { ConfigurationSource = cs };
- i++;
- }
+ else if (args[i] is "--config-source" or "--configuration-source" or "-c" && i + 1 < args.Length)
+ {
+ if (!ConfigurationSourceExtensions.TryParse(args[i + 1], out var cs, true, true))
+ throw new ArgumentException($"Invalid configuration source: {args[i + 1]}");
+ options = options with { ConfigurationSource = cs };
+ i++;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| else if (args[i] is "--config-source" or "--configuration-source" or "-c" && i + 1 < args.Length) | |
| { | |
| if (args.Length > i + 1) | |
| { | |
| cli = cli with { LogLevel = GetLogLevel(args[i + 1]) }; | |
| globalArgs.Add("--log-level"); | |
| globalArgs.Add(args[i + 1]); | |
| } | |
| i++; | |
| } | |
| else if (args[i] is "--config-source" or "--configuration-source" or "-c") | |
| { | |
| if (args.Length > i + 1 && ConfigurationSourceExtensions.TryParse(args[i + 1], out var cs, true, true)) | |
| { | |
| cli = cli with { ConfigurationSource = cs }; | |
| globalArgs.Add("--config-source"); | |
| globalArgs.Add(args[i + 1]); | |
| } | |
| if (ConfigurationSourceExtensions.TryParse(args[i + 1], out var cs, true, true)) | |
| options = options with { ConfigurationSource = cs }; | |
| i++; | |
| else if (args[i] is "--config-source" or "--configuration-source" or "-c" && i + 1 < args.Length) | |
| { | |
| if (!ConfigurationSourceExtensions.TryParse(args[i + 1], out var cs, true, true)) | |
| throw new ArgumentException($"Invalid configuration source: {args[i + 1]}"); | |
| options = options with { ConfigurationSource = cs }; | |
| i++; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Elastic.Documentation/GlobalCommandLine.cs` around lines 23 - 27, The
parsing branch for the --config-source/--configuration-source/-c flag currently
ignores invalid values because ConfigurationSourceExtensions.TryParse silently
fails; update the branch in GlobalCommandLine.cs so that if TryParse(args[i +
1], out var cs, true, true) returns false you reject the input (e.g., log a
clear error and exit or throw a UsageException) instead of leaving the default,
otherwise continue setting options = options with { ConfigurationSource = cs }
and incrementing i; this change ensures invalid config-source values are
surfaced immediately.
Comment on lines
+77
to
78
| canonicalBaseUri ??= new Uri("https://docs-v3-preview.elastic.dev"); | ||
|
|
Contributor
There was a problem hiding this comment.
Keep the canonical base URL absolute.
Dropping the Uri.TryCreate(..., UriKind.Absolute, ...) check allows relative Uri values to reach BuildContext, which can produce invalid canonical links. Please restore the absolute-URI validation for direct callers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/Elastic.Documentation.Isolated/IsolatedBuildService.cs` around
lines 77 - 78, Restore the absolute-URI validation when assigning
canonicalBaseUri in IsolatedBuildService (so BuildContext never gets relative
Uris): perform a Uri.TryCreate(inputValue, UriKind.Absolute, out var
canonicalBaseUri) (or validate the incoming canonicalBaseUri parameter with
UriKind.Absolute) and only set canonicalBaseUri to the default new
Uri("https://docs-v3-preview.elastic.dev") or the parsed Uri when the TryCreate
succeeds; ensure callers that pass invalid/relative values are rejected or fall
back to the absolute default so BuildContext only ever receives absolute Uris.
Comment on lines
87
to
+89
| var configPath = _fileSystem.Path.Join(docsFolder.FullName, "changelog.yml"); | ||
| var changelogPath = NormalizePath(changelogDir ?? "changelog"); | ||
| var bundlesPath = NormalizePath(bundlesDir ?? "releases"); | ||
| var changelogPath = changelogDir?.FullName ?? Path.GetFullPath("changelog"); | ||
| var bundlesPath = bundlesDir?.FullName ?? Path.GetFullPath("releases"); |
Contributor
There was a problem hiding this comment.
Default changelog directories are being resolved from the current working directory, not the repo docs folder.
When --changelog-dir and --bundles-dir are omitted, Lines 88-89 build <cwd>/changelog and <cwd>/releases, while changelog.yml is created under docsFolder and still points at docs/changelog / docs/releases. Running init --path /some/repo from another directory will create a mismatched layout.
Also applies to: 185-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tooling/docs-builder/Commands/ChangelogCommand.cs` around lines 87 - 89,
The default changelog and bundles directories are being resolved from the
current working directory instead of the repository docs folder; update the
logic in ChangelogCommand where configPath, changelogPath and bundlesPath are
computed so that when changelogDir or bundlesDir are null you join them to
docsFolder.FullName (e.g. use _fileSystem.Path.Join(docsFolder.FullName,
"changelog") and _fileSystem.Path.Join(docsFolder.FullName, "releases")) so the
default paths match where changelog.yml is written; apply the same fix to the
other occurrence mentioned (the block around lines 185-205) so all defaults
consistently resolve relative to docsFolder rather than cwd.
Comment on lines
+59
to
+65
| public async Task<int> ValidateLinkReference([Existing, ExpandUserProfile, RejectSymbolicLinks, FileExtensions(Extensions = "json")] FileInfo? file = null, string? path = null, CancellationToken ct = default) | ||
| { | ||
| await using var serviceInvoker = new ServiceInvoker(collector); | ||
| serviceInvoker.AddCommand(_linkIndexService, (file, path), | ||
| static async (s, collector, state, ctx) => await s.CheckWithLocalLinksJson(collector, state.file, state.path, ctx) | ||
| static async (s, collector, state, ctx) => await s.CheckWithLocalLinksJson(collector, state.file?.FullName, state.path, ctx) | ||
| ); | ||
| return await serviceInvoker.InvokeAsync(ctx); | ||
| return await serviceInvoker.InvokeAsync(ct); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full context of the InboundLinkCommands.cs file
head -100 src/tooling/docs-builder/Commands/InboundLinkCommands.csRepository: elastic/docs-builder
Length of output: 3468
🏁 Script executed:
# Search for CheckWithLocalLinksJson implementation
rg "CheckWithLocalLinksJson" --type cs -A 10Repository: elastic/docs-builder
Length of output: 4042
🏁 Script executed:
# Look for [Existing] attribute definition and usage
rg "\[Existing\]" --type cs -B 2 -A 5Repository: elastic/docs-builder
Length of output: 46
🏁 Script executed:
# Search for the default links.json path mentioned in the comment
rg "links\.json|\.artifacts/docs" --type csRepository: elastic/docs-builder
Length of output: 5175
The [Existing] attribute prevents the null case from reaching the fallback logic.
The underlying CheckWithLocalLinksJson handles null with file ??= ".artifacts/docs/html/links.json", but the command parameter's [Existing] attribute validates before the method executes. When no file is provided (null), the validation fails instead of allowing the default path. Either remove [Existing] to allow null, or refactor to provide an explicit default path at the command level.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tooling/docs-builder/Commands/InboundLinkCommands.cs` around lines 59 -
65, The [Existing] validation on the ValidateLinkReference method's file
parameter blocks null so the fallback in CheckWithLocalLinksJson never runs;
edit ValidateLinkReference to allow null by removing the [Existing] attribute
(or replace it with an attribute variant that permits null if available) so that
ValidateLinkReference(FileInfo? file = null, ...) can pass a null through to
_linkIndexService.CheckWithLocalLinksJson which will apply file ??=
".artifacts/docs/html/links.json"; ensure the parameter signature and any
related attribute usages on ValidateLinkReference are updated accordingly.
Comment on lines
+34
to
+37
| _ = collector.StartAsync(context.CancellationToken); | ||
| collector.EmitGlobalError($"Global unhandled exception: {ex.Message}", ex); | ||
| await collector.StopAsync(cancellationToken); | ||
| Environment.ExitCode = 1; | ||
| await collector.StopAsync(context.CancellationToken); | ||
| context.ExitCode = 1; |
Contributor
There was a problem hiding this comment.
Await collector startup before emitting the error.
StartAsync is fire-and-forget here, so EmitGlobalError and StopAsync can race the collector initialization and lose the report.
🔧 Proposed fix
- _ = collector.StartAsync(context.CancellationToken);
- collector.EmitGlobalError($"Global unhandled exception: {ex.Message}", ex);
- await collector.StopAsync(context.CancellationToken);
+ await collector.StartAsync(context.CancellationToken);
+ try
+ {
+ collector.EmitGlobalError($"Global unhandled exception: {ex.Message}", ex);
+ }
+ finally
+ {
+ await collector.StopAsync(context.CancellationToken);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| _ = collector.StartAsync(context.CancellationToken); | |
| collector.EmitGlobalError($"Global unhandled exception: {ex.Message}", ex); | |
| await collector.StopAsync(cancellationToken); | |
| Environment.ExitCode = 1; | |
| await collector.StopAsync(context.CancellationToken); | |
| context.ExitCode = 1; | |
| await collector.StartAsync(context.CancellationToken); | |
| try | |
| { | |
| collector.EmitGlobalError($"Global unhandled exception: {ex.Message}", ex); | |
| } | |
| finally | |
| { | |
| await collector.StopAsync(context.CancellationToken); | |
| } | |
| context.ExitCode = 1; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tooling/docs-builder/Middleware/CatchExceptionMiddleware.cs` around lines
34 - 37, Await the collector startup before emitting or stopping it: change the
fire-and-forget call to await collector.StartAsync(context.CancellationToken) so
that EmitGlobalError and StopAsync run only after StartAsync completes; ensure
you still await StopAsync afterwards and preserve setting context.ExitCode = 1,
referencing collector.StartAsync, collector.EmitGlobalError, collector.StopAsync
and context.ExitCode.
Comment on lines
+27
to
31
| var latestVersionUrl = await GetLatestVersion(context.CancellationToken); | ||
| if (latestVersionUrl is null) | ||
| ConsoleApp.LogError("Unable to determine latest version"); | ||
| _logger.LogWarning("Unable to determine latest version"); | ||
| else | ||
| CompareWithAssemblyVersion(latestVersionUrl); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/tooling/docs-builder/Middleware/CheckForUpdatesMiddleware.csRepository: elastic/docs-builder
Length of output: 4172
Wrap update check in error handling to avoid breaking successful commands
The GetLatestVersion call at line 27 can throw from file I/O or HTTP operations (lines 68, 76, 82), causing the middleware to fail the entire command despite successful execution. Additionally, HttpClient and response at lines 75–76 are never disposed, leaking resources on repeated invocations. Make the update check best-effort and dispose HTTP resources.
Proposed fix
public async ValueTask InvokeAsync(CommandContext context, CommandMiddlewareDelegate next)
{
await next(context);
if (!string.IsNullOrEmpty(Environment.GetEnvironmentVariable("CI")))
return;
- var latestVersionUrl = await GetLatestVersion(context.CancellationToken);
+ Uri? latestVersionUrl;
+ try
+ {
+ latestVersionUrl = await GetLatestVersion(context.CancellationToken);
+ }
+ catch (OperationCanceledException) when (context.CancellationToken.IsCancellationRequested)
+ {
+ throw;
+ }
+ catch (Exception e)
+ {
+ _logger.LogWarning(e, "Unable to determine latest version");
+ return;
+ }
if (latestVersionUrl is null)
_logger.LogWarning("Unable to determine latest version");
else
CompareWithAssemblyVersion(latestVersionUrl);
}
@@
- var httpClient = new HttpClient(new HttpClientHandler { AllowAutoRedirect = false });
- var response = await httpClient.GetAsync("https://github.com/elastic/docs-builder/releases/latest", ct);
+ using var httpClient = new HttpClient(new HttpClientHandler { AllowAutoRedirect = false });
+ using var response = await httpClient.GetAsync("https://github.com/elastic/docs-builder/releases/latest", ct);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tooling/docs-builder/Middleware/CheckForUpdatesMiddleware.cs` around
lines 27 - 31, GetLatestVersion can throw and HttpClient/response are not
disposed, so make the update check best-effort: wrap the call to
GetLatestVersion (and the subsequent CompareWithAssemblyVersion) in a try/catch
that logs the exception (use _logger.LogDebug or LogWarning) but does not
rethrow, ensuring the middleware does not fail the command; inside
GetLatestVersion ensure HTTP resources are disposed (use using/using var for
HttpClient and HttpResponseMessage and dispose any streams/readers used) and
catch file/IO/HTTP exceptions there as well or let them bubble to the outer try
so they are logged and ignored rather than breaking execution.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
docs-builderused ConsoleAppFramework as its CLI layer. Two fundamental problems made it worth replacing:1. Help output was nearly useless.
CAF rendered a flat list of flags with no descriptions, no namespace grouping, and no validation hints. There was no way to express that
--endpointrequires an http/https URI, that--plan-filemust exist on disk, or that--exporters Html,Elasticsearchis a comma-separated collection. Contributors had to read source code to understand what a command accepted.2. Namespace support was a string hack.
Commands were registered with prefixes like
"assembler content-source"— a flat simulation of hierarchy. There was no realassembler --helppage that showed only assembler sub-commands, no per-namespace help, and no way to document what theassemblernamespace is.What
This PR replaces ConsoleAppFramework with Nullean.Argh (0.11.0), a Roslyn source-generator CLI framework that is AOT/trim-safe by design.
Real namespace hierarchy
assembler,assembler deploy,assembler navigation,codex,changelogetc. are now first-class namespaces.docs-builder assembler --helpshows only assembler commands. Each namespace has a one-liner summary visible in the parent listing.Help text driven by XML docs
<summary>becomes the terse one-liner.<remarks>becomes the explanatory block. Param docs flow directly into flag descriptions. The migration invested in complete XML documentation for every command, with concepts like assembler, codex, link registry, and bloom filter explained inline for contributors unfamiliar with the infrastructure.--helpno longer emits host startup logs — argh 0.9.0+ detects intrinsic commands (--help,--version,__schema,__completion) at registration time and exits before the host is built.Typed, validated parameters
URIs —
--endpointand--proxy-addressareUri?with[Url](DataAnnotations); argh rejects non-http/https values at parse time.Timeouts —
--bootstrap-timeoutisTimeSpan?with[TimeSpanRange("1s","60m")]. Users pass4mor90s, not a raw integer.Thread / buffer counts —
[Range(1,128)]on thread counts,[Range(1,10_000)]on buffer size,[Range(0,20)]on retries.File and directory paths — path parameters are
FileInfoorDirectoryInfo(notstring). Help shows<file>or<dir>. Where a file must exist before the command runs (config files, cert paths, plan files, links.json),[Existing]is applied and argh rejects at parse time rather than deep inside service code.[RejectSymbolicLinks]is on every path parameter.[ExpandUserProfile]means~/pathworks everywhere.File extensions —
[FileExtensions(Extensions="yml,yaml")]on config file arguments,[FileExtensions(Extensions="json")]on JSON inputs,[FileExtensions(Extensions="pem,der,crt,cer")]on certificate paths.Exporters — previously a custom
ExporterParserwith its own vocabulary (es,config,links…). Now nativeIReadOnlySet<Exporter>binding — argh parses and validates the values, and lists the allowed set in help. Enum parsing is case-insensitive.DTOs replace flat parameter lists
Service methods previously took 12–22 individual
string?/bool?/int?parameters. This PR introducesIsolatedBuildOptions,AssemblerBuildOptions,AssemblerCloneOptions, and the already-existingElasticsearchIndexOptionsas proper records that:[AsParameters](one line in the command method)docs-builder buildas a named commanddocs-builder(no sub-command) still builds the current documentation set.docs-builder buildnow also works as an explicit named command (MapAndRootAlias).assembleconsolidationThe old
assemble(one-shot) andassembler(individual steps) namespace split is clarified:docs-builder assembleruns config-init + clone + build in one shot;docs-builder assembler clone/build/serve/index/sitemapare the individual steps.Breaking changes
--no-ai-enrichment--no-ai-enrichment(via--ai-enrichment/--no-ai-enrichment)--no-eis--eis/--no-eis--bootstrap-timeout 4(minutes as int)--bootstrap-timeout 4m(TimeSpan string)docs-builder assemble(old one-shot)docs-builder assemble(unchanged behaviour, new name in help)docs-builder codex <config>positional wasstringFileInfo— path must resolve to a real fileExporter names (
html,es,config,links) are not breaking —Enum.TryParseis case-insensitive sohtmlandHtmlboth work.argh bugs found and fixed upstream
During this migration five bugs were found and fixed in Nullean.Argh (0.7→0.11):
--help{identifier}in XML doc text emitted as C# interpolation holes in generated codebool?+bool no<Name>generating duplicate--no-*switch arms (CS8510)__evreused across multiple enum properties in the same options type (CS0136)[AsParameters]DTO property<summary>XML docs not rendered in--helpfor types in referenced assembliesOne known bug remains open:
[Existing]on a nullableFileInfo?/DirectoryInfo?fires its existence check even when the value is absent (null), causingArgumentNullExceptioninside the generated runner.Test plan
dotnet build— 0 errors, 0 warningsdocs-builder --help— root help, no startup logs,(default: build)alias showndocs-builder build --help— all options have descriptions,[existing]on--pathdocs-builder assembler --help— namespace summaries for all sub-namespacesdocs-builder assembler index --help— ES options show[schemes: http|https],[time-span-range],[range]hintsdocs-builder assembler deploy apply --help—--plan-fileshows[existing]docs-builder assembler deploy apply staging bucket missing.json— argh rejects with "path does not exist"docs-builder index --endpoint not-a-url— argh rejects with scheme errordocs-builder build --bootstrap-timeout bad— argh rejects with time-span errordocs-builder codex --help— codex concept explained in namespace summarydocs-builder changelog --help— all sub-command summaries are one terse line